Skip to content

275 - Validates documents with signed message and assertion#277

Merged
pitbulk merged 2 commits intoSAML-Toolkits:masterfrom
levelboy:275
Nov 10, 2015
Merged

275 - Validates documents with signed message and assertion#277
pitbulk merged 2 commits intoSAML-Toolkits:masterfrom
levelboy:275

Conversation

@levelboy
Copy link
Copy Markdown
Contributor

Status

READY

Migrations

NO

Description

see #275
The modification is that when we check the signature of the response, we only check the first Reference object for that signature. According to the SAML standard, a signature node can only have one Reference node within.

Related PRs

Problem seems to be introduced here: 275512a

Todos

  • Refactor xml_security to use only one XML library. At the moment it uses both nokogiri and REXML
  • This only checks the signature on the document, as it was doing previously. It would be best and correct according to the SAML standard to verify each signatures in the document.

Deploy Notes

No deploy notes.

Steps to Test or Reproduce

First commit adds relevant response document and failing test. Run rake test to see it failing.

…nature. As per the SAML specification, a signature can only have one reference node. (5.4.2 References)
@levelboy
Copy link
Copy Markdown
Contributor Author

levelboy commented Nov 5, 2015

@pitbulk Do you have any notes on this?
Thank you.

@pitbulk
Copy link
Copy Markdown
Collaborator

pitbulk commented Nov 5, 2015

@levelboy Sorry, I had no time to review it yet.

@pitbulk
Copy link
Copy Markdown
Collaborator

pitbulk commented Nov 10, 2015

Hi @levelboy,

Thanks you very much for the PR, it seems that fixs the bug introduced at 1.1.0

I read again the SAML standard and you are right, SAML expects only 1 reference

5.4.2 References
....
Signatures MUST contain a single <ds:Reference> containing a same-document reference to the ID
attribute value of the root element of the assertion or protocol message being signed. For example, if the ID attribute value is "foo", then the URI attribute in the <ds:Reference> element MUST be "#foo".

So we will apply your patch.

P.S We followed xmlsec lib steps that loop over all the references, but is true we don't need to do that on SAML:
https://github.com/robrichards/xmlseclibs/blob/1aaa0c20c9967d29a8cccdf81ad3994244c10d8d/src/XMLSecurityDSig.php#L572

@pitbulk pitbulk merged commit 9bb26bd into SAML-Toolkits:master Nov 10, 2015
@levelboy
Copy link
Copy Markdown
Contributor Author

Great, thanks @pitbulk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants